-
-
Notifications
You must be signed in to change notification settings - Fork 255
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Prevent user from wrapping already wrapped streams #201
base: master
Are you sure you want to change the base?
Prevent user from wrapping already wrapped streams #201
Conversation
ad2a750
to
2c395f8
Compare
What if user wants to The hanging and huge delays while wrapping the same stream is a big issue indeed, it has been reported in #196 and I opened #198 to fix it. Adding a safeguard arround multiple wrapping could be useful though to avoid |
2c395f8
to
9bbe4e8
Compare
Hi Delgan! This also brings another thought: if there is a test case that does a doubled initialisation, it may cause people to think that colorama may be initialised several times and the only effect should be the stream wrapper settings update. That leads to a conclusion: why calling That's also the reason why I've changed that unit test in the initial version, to avoid any further confusion in the usage of Let me add a short story that shows how you may get into troubles with several
Of course, you may say - if someone does module-level initialisation at non-singleton object level, he has a bug in his code, but I bet that pasting one of unit tests would give you an approval in most of code reviews 🙂 |
Yeah, you are probably right and your example is pretty convincible. 🙂 The fact that several people have reported encountering I can't think of any downside with your suggested change, but it should be interesting to know why it has not been implemented like this in the first time and why nested wrapping had been chosen instead. The only surprising effect would be in this case:
Then, the resulting |
Hey. FYI, Yesterday I created a PR to test releases before we push them to PyPI. When that is merged, I'll be more confident about resuming merges and releases. I'll try to look at this PR soon. Thank you for creating it! |
Due to the fact, that colorama uses global streams, multiple wraps around the same stream is pointless and causes python to hung on both Linux and Windows-based machines.
If such behavior is allowed, huge delays inside modules, that uses colorama are introduced:
Related issues:
isinitialized
function #128